fix(gui): embed plot on EnSight import, in-panel channel switching, source-agnostic cue#16
Closed
ahmadomira wants to merge 1 commit into
Closed
fix(gui): embed plot on EnSight import, in-panel channel switching, source-agnostic cue#16ahmadomira wants to merge 1 commit into
ahmadomira wants to merge 1 commit into
Conversation
…ource-agnostic cue Three issues on the plate-reader import path: 1. Plot detached from its QStackedWidget after EnSight import (overlapped the sidebar, floated above the Fit Curve tab). Cause: the EnSight path ran a modal QInputDialog mid-load, before data_loaded fired — a nested event loop during the first layout pass, unlike BMG/txt. Removing the mid-load modal makes every import path identical: parse → build → emit. 2. The missing-concentrations popup hardcoded "BMG", was verbose, and interrupted every plate-reader import. Replaced with a source-agnostic inline banner in the Data panel (names the actual format from metadata) plus auto-focus of the concentration table; the banner auto-clears once real values are entered. The fit-time guard stays but is reworded source-agnostically. The BMG-specific dialog is deleted. 3. Channels are now switchable in-panel. The reader already parses all channels into one frame; the panel keeps the full frame in memory and a Channel combo (disabled unless >1 channel) rebuilds the MeasurementSet per selection with no re-read. The concentration vector and unit carry across switches (channels share plate columns); signals, fit results and outlier state reset via the existing data_loaded handler. Keyed off the presence of a `channel` column, so any future multi-channel reader gets the dropdown for free. Adds a layout regression guard plus DataPanel tests for the combo, channel switching, and the banner.
There was a problem hiding this comment.
Pull request overview
Fixes three issues in the plate-reader GUI import path that surfaced after the EnSight reader landed: (1) removes a mid-load modal QInputDialog that was detaching the plot widget from its stacked container, (2) replaces the BMG-hardcoded modal warning with a source-agnostic inline banner derived from metadata, and (3) adds in-panel channel switching that keeps the full multi-channel frame in memory and rebuilds the MeasurementSet per selection without re-reading the file.
Changes:
DataPanel.load_fileno longer runs a modal mid-load; the panel keeps_multi_channel_dfand_channelsand exposes a Channel combo (enabled only when >1 channel) that triggers_on_channel_changedto rebuild theMeasurementSet, carrying over user-entered concentrations when no placeholder flag is set.- A new
_placeholder_bannerand_placeholder_source_nameprovide a source-agnostic inline cue;gui/dialogs/bmg_prompt_dialog.pyandFittingSession._maybe_show_bmg_promptare removed in favor of a status-bar mirror and the existing fit-time guard (also reworded source-agnostically). - New tests:
tests/unit/gui/test_fitting_session_layout.pyregression-guards plot embedding through load and channel switch;tests/unit/gui/test_data_panel.pyadds combo/banner/integration coverage including the real EnSight fixture.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
gui/widgets/data_panel.py |
Removes mid-load QInputDialog; adds multi-channel state, Channel combo, _slice_channel/_make_ms/_on_channel_changed, inline placeholder banner, source-agnostic naming helper. |
gui/fitting_session.py |
Drops _maybe_show_bmg_prompt/modal; mirrors placeholder hint in status bar; reworded source-agnostic fit-time guard. |
gui/dialogs/bmg_prompt_dialog.py |
Deleted (no remaining callers). |
tests/unit/gui/test_data_panel.py |
Adds TestChannelCombo, TestPlaceholderBanner, TestEnsightLoadIntegration covering combo enablement, channel switch behavior (signals change, entered concentrations preserved, placeholders adopted), banner show/hide and source naming, and full load_file on real fixture. |
tests/unit/gui/test_fitting_session_layout.py |
New regression: plot widget remains at stack index 0, non-window, parented by _plot_stack before/after EnSight load and channel switch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
Author
|
Squash-integrated into |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three issues on the plate-reader import path that surfaced after the EnSight reader landed. All three are GUI-layer; the reader is untouched. Stacks on
feat/reader-ensight.1. Plot detached from the tab after EnSight import
The plot overlapped the sidebar, couldn't be resized smaller, and floated above the Fit Curve tab. Cause: the EnSight path ran a modal
QInputDialogmid-load_file, beforedata_loadedfired — a nested event loop during the first layout pass, unlike the BMG/txt paths. The plot widget is never reparented in code; the modal was the only structural divergence. Fix: remove the mid-load modal so every import path is identical (parse → build MeasurementSet → emit). A new regression test (test_fitting_session_layout.py) asserts the plot stays at stack index 0 and never becomes a top-level window, across load + channel switch.2. Missing-concentrations popup → source-agnostic inline cue
The old popup hardcoded "BMG" even for EnSight, was verbose, and interrupted every import. Fix:
gui/dialogs/bmg_prompt_dialog.pyremoved; no other callers).ensight_metadata→ "EnSight",bmg_metadata→ "BMG", else generic) — never hardcodes one. It auto-focuses the concentration table and auto-clears once real values are entered.3. In-panel channel switching (no re-import)
The reader already parses all channels into one frame; the load-time picker discarded all but one. Fix: the panel keeps the full multi-channel frame in memory and adds a Channel combo (disabled unless >1 channel) that rebuilds the MeasurementSet per selection — no file re-read. The concentration vector + unit carry across switches (all channels share the same plate columns); signals, fit results, and outlier/active-replica state reset via the existing
data_loadedhandler. Keyed off the presence of achannelcolumn, so any future multi-channel reader gets the dropdown for free.Test plan
pytest tests/unit/gui/ tests/unit/test_ensight_reader.py tests/unit/test_io*.py— 128 passed, 6 skippedload_fileEnSight integration; plot-embed layout regression guard.FittingSessioncheck: plot stays embedded (stack idx 0, not a window) through EnSight load and channel switch; combo enabled with 3 channels; banner shown; 8×12 MeasurementSet.